Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(agent): normalize indentation in code completion postprocess #3361

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Sma1lboy
Copy link
Collaborator

@Sma1lboy Sma1lboy commented Nov 2, 2024

#3263

Fixing wrong indent in inline completion predict solution

Demo

Screen.Recording.2024-11-15.at.09.24.21.mov

Before

Only first line has extra spaces:
image

exactly one extra space in the line indent
image

After

image
exactly one extra space in the line indent
image

Implementation

Using whether the current indent is even as the judgment basis. If it's even, the prediction doesn't need any extra spaces and the prediction context will be processed accordingly.

Potential risks:

Some users might use odd indent size

Why

This approach is simple and doesn't require adding additional LSP or obtaining the best indent through client operations.

Issue didn't fix

Due to some concern this post-processing not going to fix this situation

All lines in prediction have extra spaces:
image

@Sma1lboy
Copy link
Collaborator Author

Sma1lboy commented Nov 4, 2024

Follow up: This is kind of a post-processing filter to prevent inconsistent extra spaces due to different model capabilities. The above bad example happens when using the demo website model, but sometimes works fine with StarCoder-1B without this modification.

@icycodes icycodes changed the title fix: normalize indentation in code completion postprocess fix(agent): normalize indentation in code completion postprocess Nov 14, 2024
@icycodes
Copy link
Member

As all bad cases in #3263 have exactly one extra space in the line indent, I suggest we should only process the case of exactly one extra space in the line indent, not extra indent.
It should be allowed to have extra indents in the completion text. Consider the case of

function foo() {
║
}

and the completion is

function foo() {
├    bar();┤
}

@Sma1lboy
Copy link
Collaborator Author

As all bad cases in #3263 have exactly one extra space in the line indent, I suggest we should only process the case of exactly one extra space in the line indent, not extra indent. It should be allowed to have extra indents in the completion text. Consider the case of

function foo() {
║
}

and the completion is

function foo() {
├    bar();┤
}

The current version should work fine with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants